Policy: Support NPDS rules with pass verdicts by jrajahalme · Pull Request #1754 · cilium/proxy

jrajahalme · 2026-02-16T19:29:07Z

Add support for NPDS pass verdicts by promoting the precedence of passed-to rules to follow that of the pass verdict rule. NPDS add a new pass_precedence field that specifies the precedence up-to which rules are ignored if the pass rule matches. pass_precedence is mutually exclusive with the deny field. Pass verdict rules can not have any L7 (e.g., HTTP) rules, as the match is solely done on the port range and the remote security identities (L3/4).

Fix precedence handling between wildcard and specific match port rules by distributing wildcard port rules to the rulesets of specific port ranges. This is cheap since the rules are referred to via shared pointers and allows to keep the logic as simple as possible.

Refactor with C++20 ranges for readability.

jrajahalme · 2026-02-25T10:11:34Z

Added the new tier_last_precedence_ field to the toString() method so that it will be included in policy dumps.

nezdolik · 2026-02-25T11:40:56Z

cilium/api/npds.proto

+  oneof verdict {
+    // Precedence after which policy evaluation should be continued at for the selected
+    // remotes_policies.
+    uint32 pass_precedence = 1;


nit: you could use proto wrappers for primitive types to be able to distinguish if field was not set. Like https://protobuf.dev/reference/java/api-docs/com/google/protobuf/UInt32Value.html

Not semantically needed here, as 0 as the pass precedence makes no sense, as there are no rules with < 0 precedence to "pass-to".

The allocation overhead due to proto wrappers in Go is a growing problem, and since there is no semantic need here I'll keep this as-is.

nezdolik · 2026-02-25T13:52:56Z

cilium/network_policy.cc

+    if (tier_last_precedence_ > 0) {
+      if (tier_last_precedence_ > precedence_) {


should be enough to have this statement?

if (tier_last_precedence_ > precedence_) {

since both fields are unsigned integers, when tier_last_precedence_ is 0 this statement will always be false:

if (tier_last_precedence_ > precedence_) {

You are right, will fix :-)

nezdolik · 2026-02-25T13:56:49Z

cilium/network_policy.h

 } // namespace Cilium
 } // namespace Envoy
+
+namespace fmt {


if this namespace is only used in impl file (network_policy.cc), is better to move it there

nezdolik · 2026-02-25T14:01:03Z

cilium/network_policy.cc

-            allowed = true;
-          }
+    if (!has_headermatches_) {
+      if (std::ranges::any_of(*http_rules_, [&](auto& r) { return r.allowed(headers); })) {


may be worth adding a check for if http_rules_ does not equal nullptr in places where is being dereferenced.

All dereferences are preceded by hasHttpRules() which does the null check. But since this was not obvious I'll refactor a bit to make this clearer.

nezdolik · 2026-02-25T14:04:44Z

cilium/network_policy.cc

-        return server_context_->getTlsContext();
-      }
-      raw_socket_allowed = true;
+  bool getServerTlsContext(Ssl::ContextSharedPtr& tls_context,


may need to rename affected methods after switching the return type to bool, since method name getSmth makes the reader to assume Smth as return type. E.g. getServerTlsContext -> hasServerTlsContext

This is still a getter, but the values are returned via the parameter references, boolean return indicating whether the values were returned or not. The current structure is tailored for this signature, so I'll keep it but add comment or two.

nezdolik · 2026-02-25T14:08:02Z

cilium/network_policy.cc

  }

+  // sort by descending precedence, retaining the original order within each precedence level
+  void sort() { sortRules(rules_); }


may be enough just to have sortRules method and call that instead of sort() ?

inlined sortRules(rules_) within sort() and removed one direct call to sortRules().

nezdolik · 2026-02-25T14:29:59Z

cilium/network_policy.cc

+    // - apply in reverse order of tiers
+    // - if all remotes are covered, promotion can happen fully in-place.
+    int tier_end = pass_rules_.size();
+    for (int tier_start : pass_rules_tier_index_ | std::views::reverse) {


this statement is little bit hard to read pass_rules_tier_index_ | std::views::reverse, the following may be is easier to comprehend:

for (int tier_start : std::reverse(pass_rules_tier_index_.begin(), pass_rules_tier_index_.end())) {

or

for (vector<int>::reverse_iterator riter = pass_rules_tier_index_.rbegin(); riter != pass_rules_tier_index_.rend(); ++riter) {

Have to think about this... I guess std::views::reverse exist to not have to do that and risk errors in the additional detail?

Keeping this as is, lets see if we get used to reading | std::views::reverse.

Proxy ID is really a port number, so we can safely limit it to 16 bits. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Pass verdict handling needs to make copies of PortNetworkPolicyRules. Change the unique pointers to a shared pointers to allow for this. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Parse the new pass_precedence field. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Improve readability with the new isRemoteWildcard helper. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Pre-process policy rules with pass verdicts on on policy updates. This involves promoting the precedence of lower tier rules based on the pass verdicts on higher tiers. Depending on the remote identities being matched, the lower tier rule may need to be split up to different sets of remote identities, or if remotes match exactly then the whole rule can be promoted to the higher tier. Splitting up, as reqiuired, increases the number of rules. We counter this by keeping track of "shadowed" identities of higher tier rules, both within a tier and between tiers, and eliminate rules that can not be matched when they become completely shadowed. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

We only looked for wildcard port match if a specific port did not find any matches. Fix this by distributing wildcard port rules to all explicit port (range) rules so that they are checked in the right precedence order. This is cheap as they are referred to by a shared pointer, so no copies of the actual PortNetworkPolicyRule's are made. Refactor policy lookups to return a RuleVerdict type. Fix empty rules with other rules semantics: A port rule with an empty rules list is an implicit allow of the lowest precedence. Retain this semantic if a pass rule is added either on the same port, or to the wildcard port. Without this fix the pass rule processing does not find the (missing) default-allow-rule and the precedence of the default allow remains at 0, while it should have been promoted due to the pass rule. test TLS with deny Harden TLS test with deny policies. Make sure raw sockets are not allowed if there is overriding deny policy. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

No functional changes, but easier to read. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

jrajahalme requested a review from a team as a code owner February 16, 2026 19:29

jrajahalme requested a review from mhofstetter February 16, 2026 19:29

jrajahalme marked this pull request as draft February 16, 2026 19:29

jrajahalme removed the request for review from mhofstetter February 16, 2026 19:29

jrajahalme force-pushed the policy-pass-verdict-support-2 branch 2 times, most recently from b501309 to 69d3e50 Compare February 18, 2026 20:56

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 69d3e50 to 032880a Compare February 24, 2026 09:38

jrajahalme marked this pull request as ready for review February 24, 2026 09:38

jrajahalme added the kind/enhancement label Feb 24, 2026

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 032880a to 29148bb Compare February 24, 2026 09:53

jrajahalme changed the title ~~Policy pass verdict support 2~~ Policy: Add pass verdict support Feb 24, 2026

jrajahalme changed the title ~~Policy: Add pass verdict support~~ Policy: Support NPDS rules with pass verdicts Feb 24, 2026

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 29148bb to 0c4698a Compare February 24, 2026 10:03

jrajahalme requested a review from nezdolik February 24, 2026 10:17

This was referenced Feb 24, 2026

Add pass precedence and verdict support for HTTP policies cilium/cilium#44509

Open

Fix tidy build #1760

Merged

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 0c4698a to 0dc6df2 Compare February 25, 2026 10:09

nezdolik reviewed Feb 25, 2026

View reviewed changes

jrajahalme added 7 commits February 25, 2026 17:56

policy: Limit proxy_id to 16 bits

bc4e07c

Proxy ID is really a port number, so we can safely limit it to 16 bits. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

policy: Make PortNetworkPolicyRule copyable

5c9a610

Pass verdict handling needs to make copies of PortNetworkPolicyRules. Change the unique pointers to a shared pointers to allow for this. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

api: Add support for pass verdict

a8caf83

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

policy: Parse pass_precedence

628c370

Parse the new pass_precedence field. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

policy: Add isRemoteWildcard()

fdac076

Improve readability with the new isRemoteWildcard helper. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 0dc6df2 to 8bf3f7a Compare February 25, 2026 17:03

jrajahalme requested a review from nezdolik February 25, 2026 17:04

policy: Refactor to simplify

018ff0d

No functional changes, but easier to read. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

jrajahalme force-pushed the policy-pass-verdict-support-2 branch from 8bf3f7a to 018ff0d Compare February 25, 2026 17:08

		if (tier_last_precedence_ > 0) {
		if (tier_last_precedence_ > precedence_) {

Uh oh!

Conversation

jrajahalme commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrajahalme commented Feb 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jrajahalme commented Feb 16, 2026 •

edited

Loading